gh-146581: Fix vulnerability in shutil.unpack_archive() for ZIP files on Windows#146591
gh-146581: Fix vulnerability in shutil.unpack_archive() for ZIP files on Windows#146591serhiy-storchaka wants to merge 6 commits intopython:mainfrom
Conversation
Use ZipFile.extractall() to sanitize file names and extract files. Files with invalid names (e.g. absolute paths) are now extracted with different names instead of been skipped or written out of the destination directory. Files containing ".." in the name are no longer skipped.
…ve-extractall' into shutil-unpack_archive-extractall
serhiy-storchaka
added
type-security
|
Wasn't |
|
Also, the new |
Backports will be fixed to use equivalent code.
Backslash is not a separator on Posix. It is a legal character which has no special meaning. |
|
I believe it’s actually a regression. Currently, In this PR it switches to a component based check, which removes that existing protection. So I believe this actually reduces the existing security strictness for non windows users |
|
There is no backslash traversal on Linux. |
|
|
||
| fp = None # Set here since __del__ checks it | ||
| _windows_illegal_name_trans_table = None | ||
| _ignore_invalid_names = False |
There was a problem hiding this comment.
Is this intended to be set by users needing back-compat as well? Should we document it (carefully)?
There was a problem hiding this comment.
It is intended for internal use in shutil.unpack_archive() only.
We could add it as optional parameter, but this does not look like a good long term solution. Even if we make the parameter name underscored, it will be visible in the help, and will confuse users. Users will start to use it, so it will be difficult to get rid of it.
I think that in future versions we can add a hook which would be called for all entry names. It would allow to translate them to valid paths, allow to skip them (with possible logging) or raise an exception. But this is a new feature, it cannot be backported. And it needs a separate discussion.
3 participants
Use ZipFile.extractall() to sanitize file names and extract files.
Files with invalid names (e.g. absolute paths) are now skipped.
Files containing ".." in the name are no longer skipped.